[🚀 사이클2 - 미션 (기물 확장 + DB 적용)] 보예 미션 제출합니다. by boyekim · Pull Request #337 · woowacourse/java-janggi

boyekim · 2026-04-08T12:24:06Z

체크 리스트

미션의 필수 요구사항을 모두 구현했나요?
Gradle test를 실행했을 때, 모든 테스트가 정상적으로 통과했나요?
애플리케이션이 정상적으로 실행되나요?

어떤 부분에 집중하여 리뷰해야 할까요

리뷰 요청이 늦어진 점 죄송합니다...

DB 적용

DB는 SQLite를 사용했습니다. 이유는 요구사항을 충족시키려면 파일 DB여야하고, H2는 이전에 예약어 때문에 귀찮았던 경험이 있어 반감이 있습니다. 그래서 한번도 안써본 SQLite를 사용해봤습니다.
DB에 어떤 정보를 저장할지 고민한 끝에, 보드 상태 전체가 아니라 이동 정보만 저장하도록 구성했습니다.
결론은 이전에 에코가 남겨주신 이동 정보만 남겨보았습니다! 이유는 어차피 애플리케이션 재시작 시 Turn 정보를 재구성하기 때문입니다. 이 방식은 보드 스냅샷을 직접 저장하는 것보다 저장 구조가 단순하고, 기존 doGame / move 흐름을 그대로 재사용할 수 있다는 장점이 있다고 판단했어요.

이전 코멘트 적용

Board를 인자로 받을 경우 Board <-> Piece 양방향 의존성이 생깁니다. Piece의 변경에 Board가 영향을 받고, Board 변경에 Piece가 영향을 받는데요, 이 부분 어떻게 해결할 수 있을까요?

해당 코멘트 적용 방식은 다음과 같습니다.
AS-IS

Piece가 Board를 직접 받아 장애물을 검증한다.
이 과정에서 Board와 Piece 사이에 양방향 의존성이 생긴다.

TO-BE

Board가 이동 경로의 위치 정보를 수집해 DTO(MoveRoute)로 만든다.
Piece는 Board 대신 MoveRoute를 받아 이동 가능 여부를 검증한다.

이 변경을 통해 Piece가 보드 전체 구조를 직접 알 필요가 없어졌고, Board와 Piece 사이의 결합도를 낮출 수 있었습니다.

echo724

안녕하세요 보예!
저도 리뷰가 늦어진 점 죄송합니다..
일단 공성과 DB 요구사항 깔끔하게 잘 처리하신 것 같아요! 기존 턴제 기반 구조에 맞게 디비에도 이동 정보 기반으로만 잘 담아서 표현하신 것 같습니다. 다른 크루들의 구조를 보면 트랜젝션을 이용해서 피스들을 전체 업데이트 해야하는 구조인데, 현재 보예 코드 기준으로는 트랜젝션이 필요하지도 않아서 좋은 설계가 아닌가 생각이 듭니다! 오히려 보드 전체를 담았다면 굳이 필요하지 않은 정보들까지 영속화가 되어 디스크에 부담을 주었을 것 같습니다. 리플레이가 된다면 굳이 보드의 스냅샷을 영속화 할 필요가 있을까요? 물론 턴이 수백턴이 넘어간다면 스냅샷을 중간중간 고려해볼 수 있겠지만 모든 턴에 보드 정보를 담는것은 어려웠을 것 같습니다. 그런 의믜에서 저는 현재 구조가 훨씬 낫다고 생각합니다.

그리고 이전 커멘트도 잘 반영해주신 것 같습니다! 굳이 피스에게 있어서 보드의 구현 내용을 알 필요는 없으니 좋은 방향으로 설계 해주신 것 같아요!

이번 리퀘스트 체인지에서는 1. Service, Game 객체들의 결합 고민 2. 서비스, 레파지토리 테스트를 추가로 해주셨으면 합니다. 1번의 경우 커멘트를 남겼고, 2번의 경우에는 디비의 경우에는 특히 배포 환경, 테스트 환경에 따라 다른 DB를 사용한다거나 하는 식으로 외부 영향을 받습니다. 1. 어떻게 하면 이 영속화 계층과 계층을 포함해서 테스트를 짤 수 있을까 2. 어떻게 하면 프로덕션 데이터베이스 없이 테스트할 수 있을지에 대해서 고민해보시고 직접 테스트 작성해보시면 좋을 것 같습니다! 혹시 애매한 부분은 커멘트 달아주시거나 DM 주세요 화이팅입니다!

echo724 · 2026-04-10T12:27:39Z

+package janggi.persistence.dto;
+
+public record MoveHistory(
+    int turnNumber,


턴 정보가 저장은 되는데 따로 쓰이는 곳은 없는 것 같네요. 혹시 넣은 이유가 있을까요?

포함해두면 유용할 것 같아서 추가했었습니다!
가장 최근 턴을 가져온다거나, 몇 번 만에 게임이 종료되었는지 등 기본 게임 정보에 해당한다고 생각해서 넣었었는데요, 코드 상에서는 사용이 안되니 제거하는게 나을까요?

아무래도 당장 구현이 된 것이 아니라면 빼는 것이 좋긴 합니다만, 구현 시점이 언제냐에 따라 다를 것 같네요. 해당 기능이 짧은 기간 내에 구현될 예정이라면 넣어도 되겠지만, 보통의 경우, 사용되지 않는 정보를 저장해도 나중에 사용되지 않을 가능성도 있고, 리펙토링을 하는 경우라면 해당 필드가 사용이 되는지 안되는지 알기 어려워 진행하기 어려워지는 부분도 있습니다!

echo724 · 2026-04-10T12:29:10Z

+    private boolean isCancelCommand(String input) {
+        return CANCEL.equalsIgnoreCase(input.trim());
    }


오 취소 커멘드 좋네요! 실제 실행해보면서 있으면 좋겠다 생각했었는데 잘 처리하셨네요!

다만 CANCEL이 JanggiRunner에 있어도 되는지에 대해서는 고민해보시면 좋을 것 같습니다. inputView에서 처리해주는게 맞지 않을까요?

제가 정의한 cancel은

이후 좌표 검증으로 들어가기 전에 취소해야함

장기 게임 규칙이라고 보기 어려움

콘솔과 사용자의 상호작용임

이런 특징을 가지고 있다고 생각했습니다.
따라서 게임과 사용자의 상호작용보다는 그 이전단계인 콘솔과의 상호작용이라고 생각해서 Runner에 위치시켰었습니다.
하지만 게임과 관련된 것이 아니니 아예 InputView가 처리해줘도 좋을 것 같네요. 오히려 Runner가 몰라도 되는 입력 규칙을 알고있게 되는 것 같아요.
inputView에 readCancelableLine라는 메서드를 추가해줘보았습니다.
어때보이나요?!

설명 감사합니다! 일단 InputView로 로직 옮기신 부분은 잘 하신 것 같네요! 다만 다른 부분에 대한 피드백은 커멘트로 남겼습니다!

echo724 · 2026-04-10T13:33:29Z

+    private void play(JanggiGame janggiGame, Position startPosition, Optional<Position> endPosition) {
+        janggiGame.doGame(startPosition, endPosition.get());
+        janggiService.saveMove(janggiGame, startPosition, endPosition.get());
+    }


장기 게임과 장기 서비스 라이프 사이클이 비슷하기도 하고, 사실 janggiGame이 Repository를 필드로 갖고 있으면 Service가 딱히 필요해보이지 않을 것 같은데 어떻게 생각하시나요? 게임이 게임 실행 + 저장을 맡아서 하는 셈인거죠

해당 구조의 장점은 게임을 실행한 다음 저장하는 간극을 줄일 수 있다고 생각합니다. 예를 들어 장기말은 인메모리 상에서 성공적으로 움직여졌는데 모종의 이유로 Service 쪽에서 저장이 안될 경우도 있잖아요? 그러면 인메모리와 영속 상태를 어떻게 일치시킬 수 있을까에 대한 고민도 필요하다고 생각합니다.

영속에 대한 책임은 기존 게임 실행과 시점이 분리되어있다고 생각해서, 게임 실행 + 저장을 분리해주려 했었습니다.

하지만 간극을 줄이는 것이 중요할 것 같아서 생각을 전개해보았는데요,
Service에서 게임 실행과 저장을 들고 있는 것은 어떨까요?
Input,Output에 대한 사용자와의 상호작용을 Runner가 하고, 게임 진행 및 저장을 Service가 해준다면 간극을 줄일 수 있고 역할도 분리할 수 있을 것 같다고 생각했습니다.
e95429f 해당 커밋에서 반영해보았습니다!

좋습니다! 해당 부분 커멘트 달았습니다~

echo724 · 2026-04-10T13:36:59Z

+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+
+class JanggiGameTest {


현재 서비스에 대한 테스트는 보이지 않네요? DB에 대한 의존성만 해결해주면 작성이 가능할 것 같은데 어떻게 해결할 수 있을까요?

Fake객체를 통해 서비스 테스트를 추가해보았습니다.
다음 두가지에서 고민했었는데요,

test용 파일 db를 판다.

fake 객체를 만든다.

하지만 관리에 대한 비용, 그리고 Service 로직만이 실패 원인이 아닐 수도 있게 될 것 같아서....우선 2번을 택해보았습니다.

echo724 · 2026-04-10T13:41:52Z

+        } catch (SQLException e) {
+            throw new IllegalStateException("수 저장에 실패했습니다.", e);
+        }


수 저장 실패에 대한 처리가 없고 예외가 터지면 종료되는 구조네요. 항상 디비 관련은 상태 불일치에 대한 고민이 필요하다고 생각하는데 게임을 종료하는 것 대신 해볼 수 있는 사항이 없을까요? 재시도를 한다던지 하는식으로요. 그리고 executeUpdate 주석에는 SQLTimeoutException이 던져진다고도 하는데 타임 아웃에 대한 예외처리도 필요하지 않을까요? 현업에서는 특히 타임아웃 관련 예외가 많이 발생하는데 이 부분도 어떻게 처리할지 또 상태 불일치는 어떻게 해결할 수 있을지 고민해보면 좋을 것 같습니다.

timeout 로직은 우선 save, update(게임 종료시 상태 변경)에서 3번까지 재시도하도록 추가해보았습니다.
상태 불일치에 대해서도 고민해보았는데, 아직은 어떤 방식이 가장 적절한지 확신이 없습니다. 특히 timeout 상황에서는 실제로 저장이 실패한 것인지, 저장은 되었지만 응답만 늦은 것인지 판단하기 어려워서 단순 재시도만으로는 해결되지 않는다고 느꼈습니다.
이 경우 사용자가 같은 이동을 다시 입력하도록 유도하는 방식도 생각해보았지만, 같은 상호작용을 다시 받는 것이 사용자 경험이나 상태 일관성 측면에서 적절한지는 조금 더 고민이 필요하다고 생각했습니다.
어떤식으로 더 개선하면 좋을지 여쭤보고싶습니다.

앗 이 커멘트를 나중에 읽었네요. 일단 타임아웃의 경우, 정말 예외상황을 제외 하고는 (서비스 리전이 다른 나라에 있다던가, 네트워크 신호가 안좋다던가) 대부분 받는쪽 대기 큐에서 기다리다가 타임 아웃이 났을 가능성이 큽니다. 보통 같은 리전에서 DB랑 서버를 운영하니 수십 밀리세컨드도 안 넘어갈 겁니다. 그래서 이런 상황에서는 리트라이를 종종 해주는 것 같아요. 대신 다른 커멘트에 남기긴 했는데 연쇄 효과가 날 수 있으니 조심해서 설정해줘야합니다. 그리고 타임아웃이 아닌 경우에는 코드대로 예외를 발생시키는게 좋을 것 같습니다! 예외가 언제 발생할지 고민해보셨으면 해서 커멘트 달았는데 생각해보면 데이터 베이스 상태가 잘못 된 경우에는 해당 예외를 던져서 더이상 유저에게 잘못된 업데이트를 하지 못하도록 프로그램을 종료시키는 것도 방법 일 것 같아요. 물론 유저가 다중 사용자일때는 또 예외 하나로 프로그램을 종료시켜도 안되지만요. 이런 부분들 고민해보시면 좋을 것 같습니다!

echo724 · 2026-04-10T13:45:59Z

+            janggiGame.findWinner()
+                .orElseThrow(() -> new IllegalStateException("승자가 존재하지 않습니다.")),
+            winnerScore


이 부분 약간 어색한데, winnerScore는 구할 수 있는데 Winner를 못찾아오는 경우가 있나요? 어차피 Turn에게서 승자 정보를 가져오는 방식이라면 묶어서 가져올 수 있는방법이 있지 않을까요?

dto로 Turn에서 묶어서 가져오도록 반영해보았습니다~~!!

echo724

안녕하세요 보예!
이번 미션 고생 많으셨습니다! 확실히 사이클 2는 공성 + 데이터베이스라 쉽지 않다는 느낌이 들었는데 그래도 마지막까지 피드백 반영 잘 해주셨네요! 기간이 지나 머지하겠습니다만 커멘트 꼭 확인해보시고 다음 미션에서 못챙긴 부분들 반영해보시거나 학습해보시면 좋을 것 같아요! 특히 데이터베이스 쪽은 앞으로도 계속 나오니 우테코 하시면서 기본기 잘 잡으시면 좋을 것 같습니다!

앞으로 미션도 화이팅입니다~

echo724 · 2026-04-14T13:20:15Z

        outputView.printAskMovePosition(janggiGame.findPiece(startPosition).nickname());
-        String rawMovePosition = inputView.readLine();
-        List<String> parsedMovePosition = DelimiterParser.parse(rawMovePosition);
+        Optional<String> rawMovePosition = inputView.readCancelableLine();


Empty(None) -> cancel, String -> move 이런식으로 구분을 하셨군요? 괜찮긴 한데, 이게 호출자 입장에서는 Optional이 기존 의미랑 다르게 사용됬다는걸 알아야하고, 이 부분 의미를 알기 위해 메서드 구현 방식을 알아야하기 때문에 가독성과 유지 보수하기 좋은 코드는 아닌 것 같습니다. 제 생각에는 코드를 읽었을때 해당 코드를 이해하기 위한 배경 지식이 필요 없고, 구현체를 볼 필요가 없는 코드가 잘 구현된 코드라고 생각하거든요. 물론 다르게 생각하실 수 있지만 이런 기준도 있다 말씀드리는 겁니다!

echo724 · 2026-04-14T13:29:04Z

+            statement.executeUpdate();
+        } catch (SQLTimeoutException e) {
+            throw new PersistenceTimeoutException("타임아웃입니다. 재시도합니다.", e);
+        } catch (SQLException e) {


타임아웃 처리 굿굿!

echo724 · 2026-04-14T13:34:05Z

+    }
+
+    private static <T> T retryOnTimeout(Supplier<T> supplier, Runnable onTimeout, int retryCount) {
+        try {


이미 아실 수 도 있고, 이 부분이랑은 크게 관련이 없을 수 도 있는데 Retry라 생각나서 적습니다~ 알고계셨다면 무시해주세요. Retry도 주의해서 설정해야하는데 연쇄 효과로 인해 부하를 줄 수 있습니다. 예를 들어, 만약 클리아언트나 다운스트림(우리 API를 사용하는 서비스)에서 retry 로직이 있고 우리쪽에서도 db에 retry가 있다고 할 경우, 각각 3번씩 리트라이를 한다고 했을때, 1번의 클라이언트 콜이 총 27번까지도 db에 요청을 줄 수 있기 때문에 주의하셔야해요. 특히 DB의 경우, 과도한 요청으로 응답시간이 느려졌을때, 이런 현상이 종종 발생할 수 있습니다!

echo724 · 2026-04-14T13:40:21Z

+            PersistenceExecutor.retryOnTimeout(
+                () -> janggiService.play(janggiGame, startPosition, endPosition.get()),
+                this::printPersistenceTimeoutMessage
+            );


살짝 아쉬울 수 있는 부분은 사실 Runner의 입장에서 Timeout에 대한 retry로직은 관심사나 책임이 아닐 수 있습니다. 오히려 Repository나 DAO의 역할에 좀 더 가까운데, print 때문에 이 부분에서 처리하려고 하신 것 같아요. 타임아웃과 리트라이 메세지를 굳이 유저에게 보여줄 필요가 있을까요? 로그로 남기면 안되나요? 만약 보여줘야한다고 생각하면 현재 구조가 맞을 것 같긴 합니다만 확실히 좀 코드가 읽이 어려워지는 부분은 있습니다!

echo724 · 2026-04-14T13:41:45Z

+            janggiGame.makeCurrentTurnBoardSnapShot(),
+            winnerResult.winner(),
+            winnerResult.winnerScore()
+        );


구웃 피드백 잘 반영해주셨네요!

echo724 · 2026-04-14T14:01:33Z

+        } catch (SQLException e) {
+            throw new IllegalStateException("수 저장에 실패했습니다.", e);
+        }


앗 이 커멘트를 나중에 읽었네요. 일단 타임아웃의 경우, 정말 예외상황을 제외 하고는 (서비스 리전이 다른 나라에 있다던가, 네트워크 신호가 안좋다던가) 대부분 받는쪽 대기 큐에서 기다리다가 타임 아웃이 났을 가능성이 큽니다. 보통 같은 리전에서 DB랑 서버를 운영하니 수십 밀리세컨드도 안 넘어갈 겁니다. 그래서 이런 상황에서는 리트라이를 종종 해주는 것 같아요. 대신 다른 커멘트에 남기긴 했는데 연쇄 효과가 날 수 있으니 조심해서 설정해줘야합니다. 그리고 타임아웃이 아닌 경우에는 코드대로 예외를 발생시키는게 좋을 것 같습니다! 예외가 언제 발생할지 고민해보셨으면 해서 커멘트 달았는데 생각해보면 데이터 베이스 상태가 잘못 된 경우에는 해당 예외를 던져서 더이상 유저에게 잘못된 업데이트를 하지 못하도록 프로그램을 종료시키는 것도 방법 일 것 같아요. 물론 유저가 다중 사용자일때는 또 예외 하나로 프로그램을 종료시켜도 안되지만요. 이런 부분들 고민해보시면 좋을 것 같습니다!

echo724 · 2026-04-14T14:02:19Z

+    private void play(JanggiGame janggiGame, Position startPosition, Optional<Position> endPosition) {
+        janggiGame.doGame(startPosition, endPosition.get());
+        janggiService.saveMove(janggiGame, startPosition, endPosition.get());
+    }


좋습니다! 해당 부분 커멘트 달았습니다~

echo724 · 2026-04-14T14:04:26Z

+    private boolean isCancelCommand(String input) {
+        return CANCEL.equalsIgnoreCase(input.trim());
    }


설명 감사합니다! 일단 InputView로 로직 옮기신 부분은 잘 하신 것 같네요! 다만 다른 부분에 대한 피드백은 커멘트로 남겼습니다!

echo724 · 2026-04-14T14:07:11Z

+package janggi.persistence.dto;
+
+public record MoveHistory(
+    int turnNumber,


아무래도 당장 구현이 된 것이 아니라면 빼는 것이 좋긴 합니다만, 구현 시점이 언제냐에 따라 다를 것 같네요. 해당 기능이 짧은 기간 내에 구현될 예정이라면 넣어도 되겠지만, 보통의 경우, 사용되지 않는 정보를 저장해도 나중에 사용되지 않을 가능성도 있고, 리펙토링을 하는 경우라면 해당 필드가 사용이 되는지 안되는지 알기 어려워 진행하기 어려워지는 부분도 있습니다!

boyekim added 17 commits April 6, 2026 21:07

feat: 궁성 기능 추가

d92271f

feat: 궁 포획 시 종료 로직 추가

f7ef7c6

feat: 말 선택 취소 기능 추가

a94521d

refactor: 사용하지 않는 메서드 제거

87ef1ba

feat: 점수 계산 기능 추가

8165335

test: 점수 계산 테스트 추가

65be702

build: build.gradle 의존성 추가

0bf8a46

feat: 스키마 파일 추가

e972bc8

feat: 데이터 베이스 생성 로직 추가

679ccd1

feat: 레포지토리 코드 추가

973782d

feat: DB 저장 기능 추가

1d1b1f9

test: 궁성 테스트 추가

348f8c7

refactor: AppConfig 추가로 객체 생성 로직 분리

ef1561c

test: 궁성 테스트 추가

e2a542e

test: 게임 종료 테스트 추가

58c3ec8

refactor: Piece의 Board 의존성 제거

2416820

test: 테스트 수정

c2fc2fc

echo724 self-requested a review April 8, 2026 12:59

echo724 requested changes Apr 10, 2026

View reviewed changes

boyekim added 5 commits April 12, 2026 17:41

refactor: cancel 처리 책임 InputView로 이관

85ba374

refactor: 게임 실행과 저장 간극 감소

e95429f

test: Service 테스트 추가

d3c641a

feat: timeout 처리 로직 추가

b9063cf

refactor: Winner 반환 로직 수정

5ea7a59

echo724 self-requested a review April 14, 2026 07:34

echo724 approved these changes Apr 14, 2026

View reviewed changes

echo724 merged commit 4db3e33 into woowacourse:boyekim Apr 14, 2026

Conversation

boyekim commented Apr 8, 2026

체크 리스트

어떤 부분에 집중하여 리뷰해야 할까요

DB 적용

이전 코멘트 적용

Uh oh!

echo724 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

boyekim Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

boyekim Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

boyekim Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

boyekim Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

echo724 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

boyekim Apr 12, 2026 •

edited

Loading

boyekim Apr 12, 2026 •

edited

Loading

boyekim Apr 12, 2026 •

edited

Loading

boyekim Apr 13, 2026 •

edited

Loading